Rust coverage report (for Suricata) #4697

catenacyber · 2020-11-24T13:13:06Z

cc @inferno-chromium and @Dor1s
Fixes #4637

This will work when rust-lang/rust#79365 gets merged into rust nightly compiler and gets shipped into oss-fuzz docker images (or if you recompile rustc)

This is a draft PR.
The right one will not use a custom patch for Suricata, but wait until OISF/suricata#5595 is merged

Last, there is a question about the demangler
As some project may have both C++ and Rust, I think that the demangler should be a custom one basically doing rustfilt | c++filt -n
Do you have thoughts on that ?

inferno-chromium · 2020-11-25T00:33:13Z

yes i think we would need add rustfilt on clusterfuzz bots ? do you have example on workflow, like is rustfilt done first ?

catenacyber · 2020-11-25T08:02:30Z

No example yet, but I guess I could try on ecc-diff-fuzzer which has both C++ and Rust

catenacyber · 2020-11-25T08:05:02Z

I hope it can be run in any order and both rustbelt and c++filt leave untouched the symbols they do not recognize

I will wait for rustc to be merged before trying this

catenacyber · 2020-12-04T11:22:00Z

@inferno-chromium this seems to work now that rustc and Suricata got updated

I will leave some comments in the code

What remains to do is to add rust in the authorized languages for coverage in python helper scripts (including CI)

catenacyber · 2020-12-04T11:23:09Z

infra/base-images/base-builder/cargo

+# we need this until https://github.com/rust-lang/cargo/issues/5450 or similar is merged
+# because cargo uses relative paths for the current crate
+# and absolute paths for its dependencies
+


Hack is to use this cargo which come first in PATH
So we can change RUSTFLAGS for this execution
then execute /rust/bin/cargo

catenacyber · 2020-12-04T11:24:21Z

infra/base-images/base-clang/Dockerfile

@@ -27,6 +27,8 @@ RUN apt-get update && apt-get install -y wget sudo && \
    rm cmake-$CMAKE_VERSION-Linux-x86_64.sh && \
    SUDO_FORCE_REMOVE=yes apt-get remove --purge -y wget sudo

+RUN apt-get install -y zlib1g-dev


This is needed because rustc uses zlib compression for the coverage section, and llvm-cov needs to decode it

catenacyber · 2020-12-04T11:24:59Z

infra/base-images/base-runner/rcfilt

+################################################################################
+
+# simply pipe
+rustfilt | c++filt -n


This seems to work for ecc-diff-fuzzer which has rust and C++

catenacyber · 2020-12-04T11:27:33Z

Tested with

python3 infra/helper.py build_image --no-pull base-clang
python3 infra/helper.py build_image --no-pull base-builder
python3 infra/helper.py build_image --no-pull base-runner
python3 infra/helper.py build_fuzzers --sanitizer=coverage suricata
python3 infra/helper.py coverage --fuzz-target fuzz_applayerparserparse --corpus-dir path/to/fuzz_applayerparserparse/ suricata
python3 infra/helper.py build_fuzzers --sanitizer=coverage ecc-diff-fuzzer
python3 infra/helper.py coverage --fuzz-target fuzz_ec_noblocker --corpus-dir path/to/corpus/ ecc-diff-fuzzer

ecc-diff-fuzzer gives a warning but report looks ok

Running fuzz_ec_noblocker
[2020-12-04 09:51:18,335 INFO] Finding shared libraries for targets (if any).
[2020-12-04 09:51:18,353 INFO] Finished finding shared libraries for targets.
warning: 2 functions have mismatched data
[2020-12-04 09:51:26,796 DEBUG] Finished generating per-file code coverage summary.

catenacyber · 2020-12-04T21:04:57Z

Ci failure seems unrelated

inferno-chromium

Thanks so much, some comments, once addressed, excited about rust cov support.

inferno-chromium · 2021-02-06T07:00:32Z

infra/base-images/base-builder/Dockerfile

@@ -149,7 +149,8 @@ RUN mkdir honggfuzz && \
    rm -rf $SRC/oss-fuzz.tar.gz

 COPY compile compile_afl compile_dataflow compile_libfuzzer compile_honggfuzz \
-    compile_go_fuzzer precompile_honggfuzz srcmap write_labels.py /usr/local/bin/
+    compile_go_fuzzer cargo precompile_honggfuzz srcmap \


nit: indent right to compile in previous line.
nit: move cargo to start, alpha order

inferno-chromium · 2021-02-06T07:01:56Z

infra/base-images/base-builder/cargo

+# because cargo uses relative paths for the current crate
+# and absolute paths for its dependencies
+
+export PATH=/rust/bin:/usr/bin


maybe just PATH="/rust/bin:$PATH" , removing current value might be problematic.

ok testing it

inferno-chromium · 2021-02-06T07:02:40Z

infra/base-images/base-runner/Dockerfile

@@ -52,6 +53,13 @@ ENV UBSAN_OPTIONS="print_stacktrace=1:print_summary=1:silence_unsigned_overflow=
 ENV FUZZER_ARGS="-rss_limit_mb=2560 -timeout=25"
 ENV AFL_FUZZER_ARGS="-m none"

+# Install Rust and cargo-fuzz for libFuzzer instrumentation.


This should be all there, so now just need rustfilt install add.

I see it in base-builder but not base-runner...

Running
docker run --rm --privileged -i -e FUZZING_ENGINE=libfuzzer -e SANITIZER=address -e RUN_FUZZER_MODE=interactive -v /path/to/whatever:/out -t gcr.io/oss-fuzz-base/base-runner /bin/bash
and then I get

cargo bash: cargo: command not found

ok fine to add then. i don't know the reason for this cargo not found.

inferno-chromium · 2021-02-06T07:02:59Z

infra/base-images/base-runner/Dockerfile

@@ -37,6 +37,7 @@ RUN apt-get update && apt-get install -y \
    python3 \
    python3-pip \
    wget \
+    curl \


Should be there already now.

Same: I see it in base-builder but not base-runner...

Should it be moved to base-image or some other generic image ?

inferno-chromium · 2021-02-06T07:03:34Z

infra/base-images/base-builder/cargo

+# limitations under the License.
+#
+################################################################################
+


Add a file description in comment.

inferno-chromium · 2021-02-06T07:05:34Z

infra/base-images/base-runner/rcfilt

+# limitations under the License.
+#
+################################################################################
+


Add file description here.

inferno-chromium · 2021-02-06T07:09:30Z

infra/base-images/base-clang/Dockerfile

@@ -27,6 +27,8 @@ RUN apt-get update && apt-get install -y wget sudo && \
    rm cmake-$CMAKE_VERSION-Linux-x86_64.sh && \
    SUDO_FORCE_REMOVE=yes apt-get remove --purge -y wget sudo

+RUN apt-get install -y zlib1g-dev


Why install this in base-clang, we avoid installing any packages as they can become dependencies of a compiled binary and might not exist in CF fuzzing time (since it has just archive). Since this is needed only for rust projects, probably add the install in coverage file itself when project language is rust.

We need llvm to be compiled with zlib support, more precisely llvm-profdata needs zlib support to read what is the output for the rust code part (because the rust compiler uses the zlib compression feature)
cf https://llvm.org/docs/CoverageMappingFormat.html (look for zlib in there)

if this is needed in llvm tools, add it here - https://github.com/google/oss-fuzz/blob/master/infra/base-images/base-clang/checkout_build_install_llvm.sh#L20 [with a comment] so that it can be removed automatically after llvm tools are compiled.

inferno-chromium · 2021-02-06T07:11:57Z

infra/base-images/base-builder/cargo

+export PATH=/rust/bin:/usr/bin
+if [ "$SANITIZER" = "coverage" ] && [ $1 = "build" ]
+then
+    abspath=`cargo metadata --no-deps --format-version 1 | jq -r '.workspace_root'`


maybe better var name than abspath, what is this abspath for?

This is the absolute path of the source code of the crate getting built.

maybe crate_src_abspath?

inferno-chromium · 2021-02-06T07:12:10Z

infra/base-images/base-builder/compile

@@ -115,7 +119,7 @@ BUILD_CMD="bash -eux $SRC/build.sh"

 # We need to preserve source code files for generating a code coverage report.
 # We need exact files that were compiled, so copy both $SRC and $WORK dirs.
-COPY_SOURCES_CMD="cp -rL --parents $SRC $WORK /usr/include /usr/local/include $GOPATH $OUT"
+COPY_SOURCES_CMD="cp -rL --parents $SRC $WORK /usr/include /usr/local/include $GOPATH /rust $OUT"


will this fail if /rust does not exist?

Likely yes
Should I use an environment variable like $GOPATH and set it appropriately ?

catenacyber · 2021-02-07T21:03:04Z

Thanks for taking the time to review it :-)
I answer quickly to some easy questions and will work the rest later

inferno-chromium

Replied to initial comments.

catenacyber · 2021-02-08T15:25:03Z

Testing with the changes, I get the following error for Suricata

Running fuzz_applayerparserparse
[2021-02-08 15:18:38,122 INFO] Finding shared libraries for targets (if any).
[2021-02-08 15:18:38,133 INFO] Finished finding shared libraries for targets.
error: fuzz_applayerparserparse: Failed to load coverage: Malformed instrumentation profile data
error: Could not load coverage information

This does not seem to be caused by LLVM version5 for covmap introduced at the end of December by https://reviews.llvm.org/D84467 as I get the same error with LLVM_REVISION=53c3acb89fcc25ba7ef1f1d76a79c241eeacb7f0 (predecessor of 9f2967bcfe2f7d1fc02281f0098306c90c2c10a5)

I shall test with different versions of the rust compiler...

inferno-chromium · 2021-02-08T15:48:27Z

Testing with the changes, I get the following error for Suricata
Running fuzz_applayerparserparse
[2021-02-08 15:18:38,122 INFO] Finding shared libraries for targets (if any).
[2021-02-08 15:18:38,133 INFO] Finished finding shared libraries for targets.
error: fuzz_applayerparserparse: Failed to load coverage: Malformed instrumentation profile data
error: Could not load coverage information
This does not seem to be caused by LLVM version5 for covmap introduced at the end of December by https://reviews.llvm.org/D84467 as I get the same error with LLVM_REVISION=53c3acb89fcc25ba7ef1f1d76a79c241eeacb7f0 (predecessor of 9f2967bcfe2f7d1fc02281f0098306c90c2c10a5)

I shall test with different versions of the rust compiler...

Thanks!

catenacyber · 2021-02-10T22:32:14Z

In facts, it looks like it is working for ecc-diff-fuzzer, and for some dummy test program, but not for Suricata...
I did not manage to find a version of Suricata working...
So I guess there is something specific in some Suricata's dependency crate that got updated since December 4th that coverage does not handle good enough...
I investigate this error Malformed instrumentation profile data
It looks like #[inline] rust functions do not get their names and we end up in llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp

if (FuncName.empty())
  return make_error<InstrProfError>(instrprof_error::malformed);

I will work for a proper fix

inferno-chromium · 2021-02-10T23:29:43Z

In facts, it looks like it is working for ecc-diff-fuzzer, and for some dummy test program, but not for Suricata...
I did not manage to find a version of Suricata working...
So I guess there is something specific in some Suricata's dependency crate that got updated since December 4th that coverage does not handle good enough...
I investigate this error Malformed instrumentation profile data
It looks like #[inline] rust functions do not get their names and we end up in llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
if (FuncName.empty())
  return make_error<InstrProfError>(instrprof_error::malformed);
I will work for a proper fix

Does it work for other rust projects, just curious ?

catenacyber · 2021-02-11T16:11:50Z

Status so far :
I use rustc 1.52.0-nightly (07194ffcd 2021-02-10)
I use my own clang version 12.0.0 (https://github.com/llvm/llvm-project.git f086e85eea94a51eb42115496ac5d24f07bc8791) with this diff

diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
index b75738bc360c..1ad5d930eafd 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -525,7 +525,8 @@ class VersionedCovMapFuncRecordReader : public CovMapFuncRecordReader {
       if (Error Err = CFR->template getFuncName<Endian>(ProfileNames, FuncName))
         return Err;
       if (FuncName.empty())
-        return make_error<InstrProfError>(instrprof_error::malformed);
+        FuncName = "lola";
+        //return make_error<InstrProfError>(instrprof_error::malformed);
       ++CovMapNumUsedRecords;
       Records.emplace_back(Version, FuncName, FuncHash, Mapping,
                            FileRange.StartingIndex, FileRange.Length);

So I can find the functions who do not get named with
/src/llvm-project/bin/llvm-cov show -name-regex="lol" -instr-profile=dump.profdata -object=/out/fuzz_applayerparserparse
after running the usual

/out/fuzz_applayerparserparse -merge=1 /tmp/dummy /path/to/corpus/
llvm-profdata merge -j=1 -sparse default.profraw -o dump.profdata

Suricata is at commit 62e665c8482c90b30f6edfa7b0f0eabf8a4fcc79

The functions who do not have names are from Rust, and hinted as inline...
For instance in num_integer crate, (file root.rs) we have this function without a name

  174|      0|fn fixpoint<T, F>(mut x: T, f: F) -> T
  175|      0|where
  176|      0|    T: Integer + Copy,
  177|      0|    F: Fn(T) -> T,
  178|      0|{

I did not manage to reproduce this with a simpler rust program using num_integer
Different versions of rustc, or changes to flags, or Suricata change the affected functions but there were always affected functions in my tries.
I guess next thing to do is to try to parse the llvm_covfun section and to investigate the function records of these functions

richkadel · 2021-02-11T19:05:45Z

@catenacyber - Thanks for pinging me about this.

I haven't seen this error before.

Let's try to find out if this is a configuration issue, unsupported compiler option (in combination with -Zinstrument-coverage) or an actual bug in the produced binary and/or coverage map.

Once we narrow it down, we should file an issue in rust-lang/rust, whether it's a bug to fix in code, or better warnings and/or documentation about unsupported configurations.

I know you said you could not reproduce the error with a simpler program, but that is going to be the best way to narrow down the issue. Perhaps you could make a copy of your code and start eliminating things in chunks that don't change the outcome (still producing the error after eliminating libraries/crates).

But there is one clue. You said this happens with functions marked as inline.

IIRC one fairly recent change (merged Jan 7) was to issue a warning when compiling with a MIR optimization level that enables inlining, and to automatically disable MIR inlining, when the -Zinstrument-coverage flag is present.

See rust-lang/rust#80521

I wonder if the error you are seeing is related to the underlying limitation.

The best guidance for this is to ensure none of your code is compiled with a MIR optimization level that enables inlining. (Essentially, you shouldn't use -Z mir-opt-level= ... when compiling with coverage enabled. )

So if you aren't enabling inlining explicitly, anywhere in your build, then I believe the inline directive would be ignored (which means it wouldn't have anything to do with the bug you're seeing). But it sure looks like it may be related, so check your build (compile with -vv if you need to) and ensure -Z mir-opt-level... is or is not being set.

If this doesn't provide the answers, we'll need a smaller example and specific code and build flags to reproduce it.

Thanks!

catenacyber · 2021-02-14T20:49:02Z

Thanks @richkadel for your answer pointing me in the right direction.
TL;DR This looks like an unsupported compiler option in combination with -Zinstrument-coverage, this option being -C opt-level=1
My reproducer code triggered the buggy behavior, but I did not compile it with this option

My simple reproducer is

use num_integer::Roots;
 
fn main() {
    let a:u32 = 54321; 
    let c = a.cbrt();
    println!("Hello, world {} {}!", a, c);
}

with Cargo.toml having

[dependencies]
num-integer = "0.1"

Compiling with
RUSTFLAGS="-Zinstrument-coverage" cargo build
and then running the target, llvm-profdata merge -j=1 -sparse and llvm-cov, it works ok

But if I compile with
RUSTFLAGS="-Zinstrument-coverage -C opt-level=1" cargo build
And then running the target, llvm-profdata merge -j=1 -sparse, then llvm-cov gives me
Failed to load coverage: Malformed instrumentation profile data
If I run my patched llvm-cov, I see the pretendedly unnamed function being fn fixpoint<T, F>(mut x: T, f: F) -> T defined in num-traits roots.rs file cf https://github.com/rust-num/num-integer/blob/master/src/roots.rs#L174

So there is no bug with -C opt-level=0 but there is a bug with -C opt-level=1

More info :

Adding -C opt-level=0 does not seem a workaround good enough for Suricata to get the right coverage information
there is no other -Z option when running cargo build -vv, neither mir option
Options such as -Zinline-mir-hint-threshold from Add flags customizing behaviour of MIR inlining rust-lang/rust#78873 seem to change the buggy functions (there are always pretendedly unnamed functions, but they are not the same depending on the value)

@richkadel what can we do next ? should I file a Rust issue with this ?

richkadel · 2021-02-14T21:45:12Z

@catenacyber - Just confirming a few things:

-O1 is a synonym for -C opt-level=1. When you searched the build commands from -vv output, did you also look for -O1 (or another digit)?
It looks like some LTO options will also force opt-level=1, as shown here. Could your build include options that trigger LTO? (That includes -lto I believe, but there may be other linker options that trigger LTO.)

If none of those options are enabled in your program, I wonder if they could be enabled in any dependent libraries you are linking with.

If you find the cause and can change a Cargo.toml build option to override when compiling with coverage, that's ideal; but I'd like to know what it is in case we can add more compile-time warnings or disable options, or both, when compiling with coverage instrumentation.

If you can't find and/or fix it without changing rustc (or cargo) then yes please file an issue.

Thank you for investigating this!

richkadel · 2021-02-14T21:46:30Z

(FYI, in case it's not clear, in -O1, the O is the capital letter o.

catenacyber · 2021-02-15T08:56:55Z

When you searched the build commands from -vv output, did you also look for -O1 (or another digit)?

As suricata's build runs cargo build --release there is -C opt-level=3 in the cargo build -vv output

If you find the cause and can change a Cargo.toml build option to override when compiling with coverage, that's ideal

Changing the option seems enough for my small reproducer, but it is not enough for my real use case
ie compiling suricata without --release and RUSTFLAGS="-C opt-level=0" (and no lto) still produces an invalid profdata file.
But in this case, it seems a bit different...
normal llvm-cov outputs Malformed instrumentation profile data but my patched llvm-cov does not show up any function

Another thing I just noticed is that the problematic functions, in addition to be hinted inline, are also called by inline functions themselves, being in traits implementations...

catenacyber · 2021-02-21T18:02:35Z

I have some new errors like

`__llvm_prf_cnts' referenced in section `.text._RNvXs5_NtCsl84xuXPsusg_6memchr4iterNtB5_7Memchr3NtNtNtNtCs3p84KrD9aKt_4core4iter6traits8iterator8Iterator4nextB7_[_RNvXs5_NtCsl84xuXPsusg_6memchr4iterNtB5_7Memchr3NtNtNtNtCs3p84KrD9aKt_4core4iter6traits8iterator8Iterator4nextB7_]' of /src/regex/fuzz/target/x86_64-unknown-linux-gnu/release/deps/libmemchr-23321bc99be58a93.rlib(memchr-23321bc99be58a93.memchr.262ojfvz-cgu.0.rcgu.o): defined in discarded section `__llvm_prf_cnts[__profc__RNvNtCsl84xuXPsusg_6memchr3x867memchr3]' of /src/regex/fuzz/target/x86_64-unknown-linux-gnu/release/deps/libmemchr-23321bc99be58a93.rlib(memchr-23321bc99be58a93.memchr.262ojfvz-cgu.0.rcgu.o)

So, looks like more debugging work is needed

richkadel · 2021-02-21T19:30:15Z

This looks very much like you still have some optimization flags in your rustc commands.

catenacyber · 2021-02-23T08:40:55Z

@richkadel I investigated this problem and I found the difference :
cargo build --manifest-path /src/regex/fuzz/Cargo.toml --release --bins works
but
cargo build --manifest-path /src/regex/fuzz/Cargo.toml --target x86_64-unknown-linux-gnu --release --bins produces this error about __llvm_prf_cnts

This seems to be because --target has an effect on RUSTFLAGS

catenacyber · 2021-02-23T09:59:57Z

@inferno-chromium I just tested this PR : it works with mp4parse-rust and rust-regex
I added a workaround for Suricata, to temporarily disable rust coverage so that we still get C coverage

One point is getting more hacky :
We use the cargo wrapper to divert call to cargo fuzz build into cd fuzz && cargo build
cargo fuzz has 2 incompatibilities for oss-fuzz :

it sets --target in its argument to cargo which results in the __llvm_prf_cnts problem. But this seems to be done on purpose by cargo fuzz
it sets --release which seems incompatible with rust coverage
Any idea there ?

By the way, the projects do not seem to use the sanitizer argument of cargo fuzz...

richkadel · 2021-02-23T15:14:13Z

@richkadel I investigated this problem and I found the difference :
cargo build --manifest-path /src/regex/fuzz/Cargo.toml --release --bins works
but
cargo build --manifest-path /src/regex/fuzz/Cargo.toml --target x86_64-unknown-linux-gnu --release --bins produces this error about __llvm_prf_cnts

This seems to be because --target has an effect on RUSTFLAGS

That is an interesting find for sure. Thanks. If that's reproducible with a simple program like the one in your bug report about -O, it would be great to have this filed as a separate issue, if you don't mind reporting it.

catenacyber · 2021-02-23T17:04:20Z

If that's reproducible with a simple program

I guess that is something like...

git clone --depth 1 https://github.com/rust-lang/regex
cd regex/fuzz
export RUSTFLAGS="-Zinstrument-coverage -C link-arg=-lc++"
cargo fuzz build -O
cargo build --manifest-path /src/regex/fuzz/Cargo.toml --release --bins

-C link-arg=-lc++ seems needed by libfuzzer

Not sure that --target x86_64-unknown-linux-gnu really makes a difference...

richkadel · 2021-02-23T17:38:02Z

@catenacyber - OK that seems easy. What is the -O for? Can you build without it?

catenacyber · 2021-02-23T20:24:16Z

What is the -O for?

for optimized ie --release (opoosite to -D for debug)

richkadel · 2021-02-23T20:54:14Z

Can you remove that flag? All optimization flags are suspicious when instrumenting for coverage.

catenacyber · 2021-02-24T09:36:42Z

@richkadel I will report this bug if I manage to reproduce it correctly in rustc

@inferno-chromium for me, this PR is ready for review (and merge if it looks ok)

richkadel · 2021-02-24T09:52:08Z

@catenacyber - I'm sorry. I think we're miscommunicating. Can you try running cargo fuzz build without the -O flag, whenever you are building with coverage enabled?

inferno-chromium · 2021-02-25T19:58:52Z

@oliverchang - can you please do the final review round.

inferno-chromium · 2021-03-06T05:16:35Z

Sorry there have been some recent changes, can you please rebase and leave comment, will merge first thing monday.

catenacyber · 2021-03-07T19:56:36Z

Sorry there have been some recent changes, can you please rebase and leave comment, will merge first thing monday.

I rebased and fixed the conflicts, so you can merge tomorrow
I also added a workaround to get Rust coverage for Suricata as well.
That is a custom patch to llvm so that llvm-cov issues a warning instead of an error in certain cases.
Coverage for Suricata's crate looks good (tested one line with print-debugging)

inferno-chromium · 2021-03-08T00:12:00Z

Sorry there have been some recent changes, can you please rebase and leave comment, will merge first thing monday.

I rebased and fixed the conflicts, so you can merge tomorrow
I also added a workaround to get Rust coverage for Suricata as well.
That is a custom patch to llvm so that llvm-cov issues a warning instead of an error in certain cases.
Coverage for Suricata's crate looks good (tested one line with print-debugging)

I dont think we can have this custom llvm patch. Reason being some day it will randomly not apply and base-builder image will just break. Can you file a upstream llvm bug with this patch and we can see if someone can help with getting it landed. Can you remove it from here, then we can land this.

catenacyber · 2021-03-08T10:08:43Z

Can you file a upstream llvm bug with this patch and we can see if someone can help with getting it landed. Can you remove it from here, then we can land this.

I removed the llvm patch.
And I replaced it with a specific workaround for Suricata (disabling coverage for one crate)
So, it should be good @inferno-chromium

alexcrichton · 2021-03-10T21:35:57Z

infra/base-images/base-builder/cargo

+    # do not optimize with --release, leading to Malformed instrumentation profile data
+    cargo build --bins
+    # copies the build output in the expected target directory
+    cd target


I think that this is causing a build failure for wasmtime because we include the fuzz directory as part of the main workspace meaning that the target directory doesn't show up under the fuzz directory.

Perhaps this could use something like:

cargo metadata --format-version 1 --no-deps | jq '.target_directory'

to extract the target directory for the fuzz project?

Another alternative, if the goal is simply to not optimize, is to set CARGO_PROFILE_RELEASE_OPT_LEVEL=0 and that should disable all optimizations without having to override Cargo?

Thanks @alexcrichton for debugging this cf #5366

Another alternative, if the goal is simply to not optimize

The goal is more than to not optimize
Indeed, we cannot optimize until rust-lang/rust#82144 is fixed
But, if I remember correctly, -Zinstrument-coverage seems incompatible with other cargo options set by cargo fuzz (like the llvm sancov pass, which would make sense)
So we turn cargo fuzz build into cargo build to be transparent to the existing build scripts

Another solution could be to have cargo fuzz accept a so-called coverage sanitizer which would set -Zinstrument-coverage
What do you think ?

I think ideally yeah it'd be best to codify this in cargo fuzz itself which could automatically apply the defaults of not optimizing and anything else necessary (like removing those extra passes)

catenacyber · 2021-03-13T17:43:09Z

All rust projects are ok for coverage build except wasmtime and libra
wasmtime should get fixed after #5366
I will check libra afterwards, maybe it fails because its build.sh set some RUSTFLAGS incompatible with coverage...

catenacyber · 2021-03-15T10:33:17Z

Libra's build was broken before coverage merge

catenacyber force-pushed the rustcov3 branch 2 times, most recently from 7363616 to c232b4a Compare December 4, 2020 11:20

catenacyber commented Dec 4, 2020

View reviewed changes

catenacyber marked this pull request as ready for review December 4, 2020 11:25

inferno-chromium mentioned this pull request Feb 3, 2021

Try to add code coverage support for Rust projects #5097

Closed

inferno-chromium requested a review from oliverchang February 3, 2021 16:09

inferno-chromium requested changes Feb 6, 2021

View reviewed changes

inferno-chromium reviewed Feb 8, 2021

View reviewed changes

catenacyber force-pushed the rustcov3 branch 2 times, most recently from 7dc9b02 to 1a82106 Compare February 8, 2021 08:32

catenacyber mentioned this pull request Feb 15, 2021

Invalid LLVM coverage data produced when compiled with -C opt-level=1 rust-lang/rust#82144

Closed

catenacyber force-pushed the rustcov3 branch from b66ba56 to 3b858fc Compare February 23, 2021 09:54

inferno-chromium marked this pull request as ready for review February 24, 2021 10:22

Rust coverage test

ca9d614

catenacyber force-pushed the rustcov3 branch from 3b858fc to 42c53ce Compare March 7, 2021 19:54

catenacyber force-pushed the rustcov3 branch from 42c53ce to d02d68e Compare March 8, 2021 09:56

Workaround to get rust coverage for Suricata

06d763c

catenacyber force-pushed the rustcov3 branch from f8a0cf1 to 06d763c Compare March 8, 2021 10:01

inferno-chromium approved these changes Mar 8, 2021

View reviewed changes

inferno-chromium merged commit c41e46f into google:master Mar 8, 2021

alexcrichton reviewed Mar 10, 2021

View reviewed changes

catenacyber mentioned this pull request Mar 11, 2021

More generic target directory for rust coverage #5366

Merged

asraa mentioned this pull request May 10, 2021

Enable fuzz testing of Rust code for project Oak #5719

Merged

Rust coverage report (for Suricata) #4697

Rust coverage report (for Suricata) #4697

Conversation

catenacyber commented Nov 24, 2020

inferno-chromium commented Nov 25, 2020

catenacyber commented Nov 25, 2020

catenacyber commented Nov 25, 2020

catenacyber commented Dec 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catenacyber commented Dec 4, 2020

catenacyber commented Dec 4, 2020

inferno-chromium left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catenacyber commented Feb 7, 2021

inferno-chromium left a comment

Choose a reason for hiding this comment

catenacyber commented Feb 8, 2021

inferno-chromium commented Feb 8, 2021

catenacyber commented Feb 10, 2021

inferno-chromium commented Feb 10, 2021

catenacyber commented Feb 11, 2021

richkadel commented Feb 11, 2021

catenacyber commented Feb 14, 2021

richkadel commented Feb 14, 2021

richkadel commented Feb 14, 2021

catenacyber commented Feb 15, 2021

catenacyber commented Feb 21, 2021

richkadel commented Feb 21, 2021

catenacyber commented Feb 23, 2021

catenacyber commented Feb 23, 2021

richkadel commented Feb 23, 2021

catenacyber commented Feb 23, 2021

richkadel commented Feb 23, 2021

catenacyber commented Feb 23, 2021

richkadel commented Feb 23, 2021

catenacyber commented Feb 24, 2021

richkadel commented Feb 24, 2021

inferno-chromium commented Feb 25, 2021

inferno-chromium commented Mar 6, 2021

catenacyber commented Mar 7, 2021

inferno-chromium commented Mar 8, 2021

catenacyber commented Mar 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catenacyber commented Mar 13, 2021

catenacyber commented Mar 15, 2021